fix(bigquery): handle API-wrapped null values in NULLABLE fields#1037
fix(bigquery): handle API-wrapped null values in NULLABLE fields#1037Srujan rai (Srujan-rai) wants to merge 2 commits into
Conversation
BigQuery returns nulls as {'v': None}, not bare None. The old check
`value is None` never matched, causing convert(None) to raise TypeError
(eg. int(None)) for NULLABLE fields. Use flatten(value) before the check.
Adds regression test for the {'v': None} API response shape.
Srujan rai (Srujan-rai)
requested review from
a team,
Kevin James (TheKevJames) and
cphoward
as code owners
Srujan rai (Srujan-rai)
requested review from
Kelly Ma (eddiedialpad) and
Shaun Sawyer (shaundialpad)
and removed request for
a team
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the parse utility to correctly handle wrapped null values from the BigQuery API by applying the flatten function to nullable fields. A unit test was added to verify this behavior. The reviewer suggested optimizing the implementation by flattening the value once at the beginning of the function to avoid redundant recursive calls and improve performance.
| if field['mode'] == 'NULLABLE' and flatten(value) is None: | ||
| return None |
There was a problem hiding this comment.
The flatten(value) function is called multiple times throughout the parse function (see lines 113, 115, 120, and 123). To improve efficiency and avoid redundant recursive traversals, consider flattening the value once and reusing the result. Since flatten is idempotent for already-flattened values, this change is safe and improves performance, especially when processing large query results with many nullable fields.
| if field['mode'] == 'NULLABLE' and flatten(value) is None: | |
| return None | |
| value = flatten(value) | |
| if field['mode'] == 'NULLABLE' and value is None: | |
| return None |
There was a problem hiding this comment.
Applied value is now flattened once before all checks. All unit tests pass.
Addresses review feedback: flatten value once before all mode/type checks instead of calling flatten(value) redundantly on every branch.
1 participant
BigQuery returns nulls as {'v': None}, not bare None. The old check
value is Nonenever matched, causing convert(None) to raise TypeError (eg. int(None)) for NULLABLE fields. Use flatten(value) before the check.Adds regression test for the {'v': None} API response shape.
Summary
{'v': None}, not bareNonevalue is Nonenever matched → fell through toconvert(None)→TypeError(eg.int(None)) for any NULLABLE INTEGER/BOOLEAN/FLOAT/etc fieldwith a null value
flatten(value) is Nonebefore the null check, consistent with how the rest ofparse()already usesflatten(){'v': None}API response shape for all NULLABLE typesFixes #577